Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Bug: LoadBalancers - Use Hostnames in addition to IP #187

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

andrewstucki
Copy link
Contributor

Changes proposed in this PR:

This adds hostname values to addresses for a loadbalancer. In particular this is necessary for AWS EKS. In EKS LoadBalancers get "ExternalP" values that aren't actually IP addresses, but rather are hostnames. This is accounted for in the underlying Service.Status.LoadBalancer.Ingress[*].Hostname fields.

The problem is two-fold. In EKS LoadBalancer have hostnames rather than ips, so we:

  1. Were never getting a valid ExternalIP value for the gateway address, and
  2. Since we didn't have an empty string check on the IP, were attempting to append an empty value onto the gateway addresses.

This was resulting in the following error:

2022-04-29T03:20:58.298Z [ERROR] k8s/logger.go:23: consul-api-gateway-server.controller-runtime.controller.gateway: Reconciler error: name=selling-fears-gateway namespace=default reconciler group=gateway.networking.k8s.io reconciler kind=Gateway
  error=
  | 1 error occurred:
  | 	* Gateway.gateway.networking.k8s.io "selling-fears-gateway" is invalid: status.addresses.value: Invalid value: "": status.addresses.value in body should be at least 1 chars long

The fix is to:

  1. Guard all of our address appending with an empty string check, and
  2. Use both IP and Hostname values for any LoadBalancer ingress

How I've tested this PR:

Unit tests and verified the bug and fix on EKS with a test image pushed to public.ecr.aws/d1c7c4d0/testing123:1.

@andrewstucki andrewstucki added type/bug Something isn't working backport-0.2.x labels Apr 29, 2022
@andrewstucki andrewstucki requested a review from a team April 29, 2022 03:41
@andrewstucki andrewstucki merged commit 5986efb into main Apr 29, 2022
@andrewstucki andrewstucki deleted the loadbalancer-hostname-fix branch April 29, 2022 11:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants